Fixes #264 #268
Merged
Fixes #264 #268
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
With a watchout, a json duplication may not be the best way to do this with typescript as it assumes this object is a data model.
p2004a
reviewed
Sep 22, 2024
| @@ -59,6 +63,13 @@ function kickBot() { | |||
| props.battle.removeBot(props.bot); | |||
| } | |||
|
|
|||
| // Duplicates this bot and its settings and gives it a new player id. | |||
| function duplicateBot() { | |||
| let duplicatedBot = JSON.parse(JSON.stringify(props.bot)); | |||
There was a problem hiding this comment.
structuredClone should be cleaner in what you are trying to do here vs pasing object via JSON serialization. It also should be better for typing.
Also change let to const.
There was a problem hiding this comment.
Thanks! Adjustments made in a877be / 959a9fc, alongside a fix for options not being set by some sort of Id.
…e rather than playerId (resulting in the abiltiy to not uniquely set options for susequent bots).
Jazcash
reviewed
Sep 22, 2024
Comment on lines
93
to
105
| // Returns a bot by it's PlayerId. Returns undefined if not found.. | ||
| public getBotParticipantByPlayerId(playerId: number): User | Bot | undefined { | ||
| return this.participants.value.find((participant) => { | ||
| const isBot = !("userId" in participant); | ||
| if (isBot) { | ||
| if (participant.playerId === playerId) { | ||
| return true; | ||
| } | ||
| } | ||
| return false; | ||
| }); | ||
| } | ||
|
|
There was a problem hiding this comment.
I think this should .find on this.bots instead of this.participants, then you can remove User from the return type
There was a problem hiding this comment.
Thanks for the suggestion!
Addressed in 7bcc76.
Jazcash
approved these changes
Sep 22, 2024
p2004a
reviewed
Sep 29, 2024
| @@ -90,6 +90,16 @@ export abstract class AbstractBattle<T extends BattleOptions = BattleOptions> { | |||
| }); | |||
| } | |||
|
|
|||
| // Returns a bot by it's PlayerId. Returns undefined if not found.. | |||
| public getBotByPlayerId(playerId: number): Bot | undefined { | |||
| return this.bots.find((bot) => { | |||
There was a problem hiding this comment.
This could have been just (bot) => bot.playerId === playerId but whatever.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implemented as a dropdown option on a bot participant as shown below, requires validation by @drivver44 as if he feels this is enough for now (perhaps last added AI right click could be an additional issue built from this?).
Two watchouts:
Please feel free to let me know if any of this is incorrect or if I can do anything better, this is my first contribution for this project and felt I could add a very little bit of value with a simple issue.